Styling focus page to similar design #23
Conversation
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
nagarajnetcentric
changed the title
|
|
|
||
| const content = `<h2>SPECIFICATIONS</h2><div><p>Learn more about the ${document.querySelector('h1').textContent} and its technical specifications.</p></div> | ||
| <table class="spec-table"><tr><th>length</th><th>width</th><th>height</th><th>weight</th></tr> | ||
| <tr><td>${specs.Length}</td><td>${specs.Width}</td><td>${specs.Height}</td><td>${specs.Weight}</td></tr><table></div>`; |
There was a problem hiding this comment.
doesn't a table restrict us quite a bit when doing a mobile version?
There was a problem hiding this comment.
Are we going to support mobile version in future?
scripts/scripts.js
Outdated
| addSpeedInformation(specificationsObj.Range, infoContainer); | ||
| // Temp content as it is not received from document | ||
| addSpeedInformation('570 light years', infoContainer); | ||
| addSpeedInformation('2.6 Sec', infoContainer); |
There was a problem hiding this comment.
as discussed let's take length and number of passengers instead
There was a problem hiding this comment.
Will change it in few mins
There was a problem hiding this comment.
Updated but some styling should be done
| document.body.dataset.features = specification.features; | ||
| document.body.dataset.specification = specification.specifications; | ||
| } catch (e) { | ||
| console.error('could not load specifications', e); |
There was a problem hiding this comment.
please add the eslint ignore here
styles/styles.css
Outdated
|
|
||
| /* stylelint-disable-next-line no-descending-specificity */ | ||
| .default-content-wrapper .spec-container p { | ||
| margin-left: 0 !important; |
There was a problem hiding this comment.
do we really need important?
There was a problem hiding this comment.
Yes because it is been overridden by other stylebody.ship-focus .default-content-wrapper p:not(p ~ p) .
There was a problem hiding this comment.
don't we do other viewports too?
There was a problem hiding this comment.
Question, are we supporting other view ports?
styles/styles.css
Outdated
| } | ||
|
|
||
| .default-content-wrapper h3:nth-child(odd) { | ||
| float: right; |
There was a problem hiding this comment.
shouldn't we rather use grid or flex instead of floating?
There was a problem hiding this comment.
We need to check as there is no block or parent element.
Test URLs: